Skip to content

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 28, 2021

Best reviewed one commit at a time.

@bjorn3 bjorn3 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-driver Area: rustc_driver that ties everything together into the `rustc` compiler labels Mar 28, 2021
@rust-highfive
Copy link
Contributor

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 28, 2021
@rust-log-analyzer

This comment has been minimized.

@bors

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 30, 2021

Rebased and added two extra commits.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 30, 2021

ff2af40cd7d4d3373bfdc247f2ad8847213aea51 should be functionally identical. Can it be a spurious error?

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2021
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a few comments.
The CI will re-run with the additional commits. We will see then if the failure is spurious.

bjorn3 added 2 commits May 2, 2021 18:00
This allows removing a confusing mem::replace in create_global_ctxt
@bjorn3 bjorn3 force-pushed the driver_cleanup branch from bb61699 to 95a1cf7 Compare May 2, 2021 16:02
@bjorn3 bjorn3 force-pushed the driver_cleanup branch from 95a1cf7 to b71cd56 Compare May 2, 2021 16:12
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member Author

bjorn3 commented May 3, 2021

CI passed.

@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented May 11, 2021

📌 Commit 163b480 has been approved by cjgillot

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 11, 2021
@bors
Copy link
Collaborator

bors commented May 12, 2021

⌛ Testing commit 163b480 with merge ac923d9...

@bors
Copy link
Collaborator

bors commented May 12, 2021

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing ac923d9 to master...

@pnkfelix
Copy link
Contributor

Performance triage indicates that this PR introduced a 1.7% regression when fully compiling coercions-debug.

I don't think any performance hit was expected. However, it also seems like this may be just noise.

@@ -967,7 +967,7 @@ pub struct GlobalCtxt<'tcx> {
export_map: ExportMap<LocalDefId>,

pub(crate) untracked_crate: &'tcx hir::Crate<'tcx>,
pub(crate) definitions: &'tcx Definitions,
pub(crate) definitions: Definitions,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change I can think of that could caused the regression if it isn't just noise.

bjorn3 pushed a commit to bjorn3/rust that referenced this pull request May 27, 2021
rustc_driver cleanup

Best reviewed one commit at a time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-driver Area: rustc_driver that ties everything together into the `rustc` compiler C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants